-
Notifications
You must be signed in to change notification settings - Fork 126
Stat implementation for Swift System #256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Note that the implementation uses typed throws which require Swift 6.0+, so earlier Swift versions are failing. |
Package.swift
Outdated
|
|
||
| let cSettings: [CSetting] = [ | ||
| .define("_CRT_SECURE_NO_WARNINGS", .when(platforms: [.windows])), | ||
| .define("_GNU_SOURCE", .when(platforms: [.linux])), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vaguely remember that this may cause an issue with the glibc module if another package imports it without this define? I.e. either everyone has to import it this way or no one can (in which case, we really want the C importer to set this instead of setting it in the package). @DougGregor?
266636c to
2f0f045
Compare
|
I would really like native TimeSpec and TimeVal wrappers to be provided by this API and conversions to/from duration be based on them. These type represent the granularity the system thinks in and papering over that with |
The stdlib already provides initializers to convert |
2f0f045 to
2f5b12b
Compare
|
@swift-ci please test |
|
@swift-ci please test |
| // MARK: - fstatat Flags | ||
|
|
||
| @_alwaysEmitIntoClient | ||
| internal var _AT_SYMLINK_NOFOLLOW: CInt { AT_SYMLINK_FOLLOW } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| internal var _AT_SYMLINK_NOFOLLOW: CInt { AT_SYMLINK_FOLLOW } | |
| internal var _AT_SYMLINK_NOFOLLOW: CInt { AT_SYMLINK_NOFOLLOW } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, thanks! Fixed in 90dba1d
Sources/System/FileSystem/Stat.swift
Outdated
| /// Total size, in bytes | ||
| /// | ||
| /// The semantics of this property are tied to the underlying C `st_size` field, | ||
| /// which can have file system-dependent behavior. For example, this property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /// which can have file system-dependent behavior. For example, this property | |
| /// which can have filesystem-dependent behavior. For example, this property |
We use the one-word "filesystem" elsewhere, so this seems like a typo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from where I used "filesystem" below, it actually looks like all prior uses in System are written "file system" or capitalized "FileSystem" (except for one), so I settled on "file system" in eee0145.
After some research, I did find that "file system-dependent" is not correct grammar and instead should be written "file-system-dependent" or "file-system–dependent" (with an en dash in the second gap) if we want to match the use of "file system." I've opted for "file-system–dependent" since it's a neat way to use an en dash to highlight the grouping of "file-system" in a larger compound modifier, but I don't feel super strongly about it if we want to change it (or even replace all uses with "filesystem") 😄
Sources/System/FileSystem/Stat.swift
Outdated
| } | ||
| #endif | ||
|
|
||
| // TODO: Investigate changing time properties to UTCClock.Instant once available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove the commented-out lines in a standalone commit so that they're easy to find later, instead of leaving commented-out code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! 501bb7d
|
@swift-ci please test |
This PR adds a Swift
Statimplementation for the Cstattypes and system calls.See the pitch at https://forums.swift.org/t/pitch-stat-types-for-swift-system/81616
See the full proposal in #257